-
Notifications
You must be signed in to change notification settings - Fork 433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added 2 new embedding models to Model.cs #193
base: master
Are you sure you want to change the base?
Conversation
Added 2 new embedding models text-embedding-3-small and text-embedding-3-large
We should probably also update the embedding endpoint helper functions to add an optional model parameter (currently it's not one of the parameters because there's only been one model), as well as update the embedding request to support dimensions. But adding these models is a good start. |
True! I only added what I needed, and the new models were enough for me, as the model can already be stated in the constructor of the EmbeddingRequest. I'm on holiday for the next week, but if you like I can have a look and enhance the helper functions and also the dimensions. |
Yeah, I think it’s probably best to leave the current default which is
important for backwards compatibility. OpenAI has explicitly stated they
won’t be deprecating that old model.
Perhaps I need to make this more clear on the documentation, but you can
always provide a text string for the model name rather than one of the
strongly typed models. So whenever OpenAI releases a new model, you can
just pass in the model name without having the modify the SDK at all.
Anyway it’s a pretty hectic weekend for me, but I’ll do my best to double
check all this and merge it early in the week.
…-Roger
On Wed, Feb 7, 2024 at 2:13 PM wrzr123 ***@***.***> wrote:
True! I only added what I needed, and the new models were enough for me,
as the model can already be stated in the constructor of the
EmbeddingRequest. I'm on holiday for the next week, but if you like I can
have a look and enhance the helper functions and also the dimensions.
I also thought about changing the default model to text-embedding-3-small,
as the ada-model will probably become deprecated in a while. But this would
be a problem with backwards compatibility for many users I guess.
—
Reply to this email directly, view it on GitHub
<#193 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJBNJCV4LQEDUP2ICUSNXDYSP4CFAVCNFSM6AAAAABC5TILSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZTGAZDIOBQHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Added 2 new models text-embedding-3-small and text-embedding-3-large Added model as optional parameter in embedding endpoints Added dimensions and user in embedding requests
Accidentally leaked API-key is already disabled :)
Alright, I made a new commit which includes
I've already found the option to initialize a custom model, was easy to find when looking at the code base. As I don't want to use customized libraries, I did it that way in my codebase which uses your nuget package. But of course I'm happy to update soon to the new version once available. ;) One more question regarding running the tests. I've let the embeddings tests run locally, to make sure I didn't break anything. To make it work, I manually inserted my OpenAI API key in the constructors. And of course I forgot to take it out again before committing... I'm glad OpenAI and Github both notified me right away about the issue. |
Added 2 new embedding models text-embedding-3-small and text-embedding-3-large